-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Added rqt_console to devroute.yaml #587
Added rqt_console to devroute.yaml #587
Conversation
WalkthroughThe pull request modifies the Changes
Assessment against linked issues
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
build/docker-compose.devroute.yaml (2)
15-15
: LGTM! Consider adding error handling for rqt_console.The addition of
rqt_console
as a background process is correctly implemented. However, consider adding error handling to ensure the service continues even ifrqt_console
fails to start.- command: bash -c "sleep 10 && sudo chown -R ${USER_UID}:${USER_GID} ../ && sudo chmod -R a+w ../ && (rqt_console &) && python3 /opt/leaderboard/leaderboard/leaderboard_evaluator.py --debug=0 --routes=$${ROUTE} --agent=/workspace/code/agent/src/agent/agent.py --host=$${CARLA_SIM_HOST} --track=MAP" + command: bash -c "sleep 10 && sudo chown -R ${USER_UID}:${USER_GID} ../ && sudo chmod -R a+w ../ && (rqt_console || echo 'Warning: rqt_console failed to start' >&2) & python3 /opt/leaderboard/leaderboard/leaderboard_evaluator.py --debug=0 --routes=$${ROUTE} --agent=/workspace/code/agent/src/agent/agent.py --host=$${CARLA_SIM_HOST} --track=MAP"
15-15
: Consider refactoring the complex command into a startup script.The command is becoming long and complex, which could make it harder to maintain. Consider moving it to a dedicated startup script.
Example structure:
+ # Create a new file: build/scripts/agent-startup.sh + #!/bin/bash + set -e + + # Wait for dependencies + sleep 10 + + # Set permissions + sudo chown -R ${USER_UID}:${USER_GID} ../ + sudo chmod -R a+w ../ + + # Start rqt_console + (rqt_console || echo 'Warning: rqt_console failed to start' >&2) & + + # Start leaderboard evaluator + exec python3 /opt/leaderboard/leaderboard/leaderboard_evaluator.py \ + --debug=0 \ + --routes=${ROUTE} \ + --agent=/workspace/code/agent/src/agent/agent.py \ + --host=${CARLA_SIM_HOST} \ + --track=MAPThen in docker-compose:
- command: bash -c "sleep 10 && sudo chown -R ${USER_UID}:${USER_GID} ../ && sudo chmod -R a+w ../ && (rqt_console &) && python3 /opt/leaderboard/leaderboard/leaderboard_evaluator.py --debug=0 --routes=$${ROUTE} --agent=/workspace/code/agent/src/agent/agent.py --host=$${CARLA_SIM_HOST} --track=MAP" + command: /workspace/build/scripts/agent-startup.sh
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
build/docker-compose.devroute.yaml
(1 hunks)
🔇 Additional comments (1)
build/docker-compose.devroute.yaml (1)
15-15
: Verify if the sleep duration is sufficient.
The command includes a 10-second sleep, presumably waiting for dependencies. Let's verify if this duration is adequate.
✅ Verification successful
10-second sleep appears to be a consistent pattern across distributed setups
The verification shows that the 10-second sleep duration is consistently used across multiple docker-compose configurations:
docker-compose.leaderboard.yaml
docker-compose.leaderboard-distributed.yaml
docker-compose.devroute.yaml
docker-compose.devroute-distributed.yaml
All these files use the same sleep duration for similar initialization tasks (file permissions setup and ROS-related operations). Since this pattern has been established across different deployment scenarios and there are no documented startup timing issues in the codebase, the 10-second duration appears to be sufficient.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any timing-related configurations or sleep commands in other compose files
echo "Checking referenced compose files for timing configurations..."
rg -A 2 "sleep|delay|timeout" roscore_service.yaml docker-compose.carla-simulator.yaml
echo "Checking if there are any documented startup requirements..."
fd -g "README*" -X rg -i "startup|boot|wait|timing"
Length of output: 545
Script:
#!/bin/bash
# Let's first find the correct docker-compose files
fd -e yaml -e yml | rg -i "docker-compose|compose"
# Then search for timing-related configurations in all compose files
echo -e "\nSearching for timing patterns in compose files:"
fd -e yaml -e yml | rg -i "docker-compose|compose" | xargs rg -A 2 "sleep|delay|timeout"
# Look for any startup documentation in the codebase
echo -e "\nSearching for startup documentation:"
rg -i "startup|boot|wait|timing" -g "!*.{pyc,so,o}" -g "!{build,dist,node_modules}/*"
Length of output: 18216
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good
Description
Added command to open up the rqt console when compose up is executed on the devroute.yaml.
Fixes #586
Type of change
Does this PR introduce a breaking change?
No
Most important changes
docker-compose.devroute.yaml
Checklist:
Summary by CodeRabbit
rqt_console
alongside the leaderboard evaluator script for improved functionality.